Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore x0 y0 in dfs2 #544

Merged
merged 22 commits into from
Feb 15, 2023
Merged

Ignore x0 y0 in dfs2 #544

merged 22 commits into from
Feb 15, 2023

Conversation

jsmariegaard
Copy link
Member

Dfs2 (and Dfs3) have both a file origin and spatial axis with a starting point x0, y0 (z0) - this can be confusing. Different MIKE tools have different ways of handling the origin and x0 y0 and in many cases x0, y0 is simply ignored. Spectral dfs2 files are, however, an exception to this rule - for these files x0 is important and must be read. The current MIKE IO implementation uses both origin and x0-y0, but this apparently has some problems as reported in #538 and #540.

This PR aims at fixing the problems mentioned in these issues by largely ignoring x0, y0 except for spectral files.

mikeio/spatial/grid_geometry.py Outdated Show resolved Hide resolved
@@ -296,6 +308,22 @@ class Grid2D(_Geometry):
_orientation: float
is_spectral: bool

def __eq__(self, other):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dataclasses already implements a useful __eq__ method by default. Is this necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, we should probably remove it again. It's just that one of the dfs3 tests failed due to _origin:

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ecomodeller , do you know why this happens in this case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'll try to find out, useful to understand.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I supppose this error is raised because, even though we claim with type hints that origin is a tuple of two floats, at runtime it is a np.ndarray which doesn't implement an __eq__ method that adhere to the behaviour that dataclasses expect.

And even if it did, the numbers are not identical due to rounding errors.

So the question is, is that rounding error (presumably caused by conversion from single to double precision) a bug or a feature?

>>> ds.geometry.origin
array([ 586422.9462332, 6142644.1548212])
>>> dsnew.geometry.origin
array([ 586422.9462332, 6142644.1548213])
>>> ds.geometry.origin == dsnew.geometry.origin
array([ True, False])

@jsmariegaard
Copy link
Member Author

closes #538, closes #540

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants